Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

src/MiniMap/miniMap.js (8)

1-5: Declare globals or import dependencies to avoid ReferenceErrors in bundled builds.

You reference ol, proj4, THREE, and $ without imports. If they’re not guaranteed globals at runtime, this will throw. At minimum, declare globals for linting or import them as modules where applicable.

Apply a minimal lint pragma:

+/* global ol, proj4, THREE, $ */
 /**
  * Initializes the Potree minimap to work with offline map and EPSG:4978 coordinates.
  *
  * @param {Potree.Viewer} viewer - The Potree viewer instance
  */

If you rely on globals provided by Potree script tags, please confirm they are always present before this module executes. Otherwise, switch to explicit imports.

---

17-20: Avoid hard‑coding the map center; derive it from the current camera via toMap with a safe fallback.

This keeps the minimap aligned with the actual dataset location and works in EPSG:4978.

-  // Transform minimap center [lon, lat] from EPSG:4326 to EPSG:3857
-  const center = proj4('EPSG:4326', 'EPSG:3857', [2, 69])
-  map.getView().setCenter(center)
+  // Center the minimap on the current camera position (EPSG:4978 → map projection), fallback to a static center.
+  const camera = viewer.scene.getActiveCamera()
+  let center
+  try {
+    const forward = viewer.mapView.toMap.forward([
+      camera.position.x,
+      camera.position.y,
+      camera.position.z
+    ])
+    if (Array.isArray(forward) && Number.isFinite(forward[0]) && Number.isFinite(forward[1])) {
+      center = [forward[0], forward[1]]
+    }
+  } catch {}
+  if (!center) {
+    center = proj4('EPSG:4326', 'EPSG:3857', [2, 69])
+  }
+  map.getView().setCenter(center)

---

28-45: Make layer replacement robust; don’t rely on magic indices.

Assuming OSM is layers.item(0) and “extent” is item(6) is brittle and can break across Potree/OL versions or config changes.

-function replaceMiniMapLayers(layers) {
-  // Remove the old Open Street Map layer
-  const oldOSMLayer = layers.item(0)
-  layers.remove(oldOSMLayer)
+function replaceMiniMapLayers(layers) {
+  // Remove the OSM base layer by type (more robust than assuming index 0)
+  const baseLayer = layers.getArray().find(
+    (l) => l instanceof ol.layer.Tile &&
+           l.getSource && l.getSource() instanceof ol.source.OSM
+  )
+  if (baseLayer) {
+    layers.remove(baseLayer)
+  }
 
   // Add new offline GeoJSON layer
   const newGeojsonLayer = new ol.layer.Vector({
     source: new ol.source.Vector({
-      url: '/data/geo/world_simplified.geojson',
+      url: '/data/geo/world_simplified.geojson',
       format: new ol.format.GeoJSON()
     })
   })
   layers.insertAt(0, newGeojsonLayer)
 
-  // Remove the extent layer
-  const extentLayer = layers.item(6)
-  layers.remove(extentLayer)
+  // Remove the extent layer if present (guard against missing/shifted index)
+  const extentLayer = layers.item(6)
+  if (extentLayer) {
+    layers.remove(extentLayer)
+  }
 }

If the “extent” layer has an identifiable property (e.g., name/title), prefer selecting it by that property instead of index.

---

35-38: Asset URL should be base‑path safe.

An absolute /data/... path breaks when the app is served under a sub‑path. Build a URL relative to the app base.

-      url: '/data/geo/world_simplified.geojson',
+      url: `${document.baseURI.replace(/\/$/, '')}/data/geo/world_simplified.geojson`,

Alternatively, if you use a bundler that supports it, prefer:

• Vite: new URL('../../data/geo/world_simplified.geojson', import.meta.url).href
• Webpack: import url from '.../world_simplified.geojson?url'
  Please pick the variant that matches your toolchain.

---

52-61: Remove both “D” and “T” controls; avoid brittle constructor.name checks.

constructor.name can change with minification and doesn’t catch the “T” button. Remove by class and by UI title/text heuristics.

-function removeTileTools(map) {
-  map.getControls().forEach((control) => {
-    if (
-      control.constructor &&
-      control.constructor.name === 'DownloadSelectionControl'
-    ) {
-      map.removeControl(control)
-    }
-  })
-}
+function removeTileTools(map) {
+  const toRemove = []
+  map.getControls().forEach((control) => {
+    const ctor = control && control.constructor && control.constructor.name
+    const el = control && control.element
+    const title = el && (el.title || (el.getAttribute && el.getAttribute('title')))
+    const text = el && el.textContent && el.textContent.trim()
+    if (
+      ctor === 'DownloadSelectionControl' ||
+      ctor === 'TileListControl' || // covers a common name for the "T" tool
+      (title && /download|tile/i.test(title)) ||
+      (text && /^[DT]$/.test(text))
+    ) {
+      toRemove.push(control)
+    }
+  })
+  toRemove.forEach((c) => map.removeControl(c))
+}

If you know the exact control classes Potree registers for D/T, prefer instanceof checks against those classes.

---

68-71: Preserve original mapView.update and fallback on errors to avoid regressions.

Monkey‑patching without a fallback can break the minimap if a transform fails. Keep the original update and call it on exceptions.

-function overrideMapViewUpdate(viewer) {
-  const Vector3 = THREE.Vector3
-  const Vector2 = THREE.Vector2
-  viewer.mapView.update = function (delta) {
+function overrideMapViewUpdate(viewer) {
+  const Vector3 = THREE.Vector3
+  const Vector2 = THREE.Vector2
+  const originalUpdate = viewer.mapView.update.bind(viewer.mapView)
+  viewer.mapView.update = function (delta) {
+    try {
       if (!this.sceneProjection) {
         return
       }
@@
-    this.gCamera.setCoordinates([p1, p2, p3, p1])
-  }
+      this.gCamera.setCoordinates([p1, p2, p3, p1])
+    } catch (e) {
+      // Fallback to original behavior to keep the minimap usable
+      if (typeof originalUpdate === 'function') {
+        return originalUpdate(delta)
+      }
+    }
+  }
 }

Also applies to: 125-126

---

76-87: Avoid per‑frame jQuery and extra DOM queries in the render loop.

This runs every update; prefer the map target element’s client size and cache lookups.

-    let pm = $('#potree_map')
-
     if (!this.enabled) {
       return
     }
 
-    let mapSize = this.map.getSize()
-    let resized = pm.width() !== mapSize[0] || pm.height() !== mapSize[1]
-    if (resized) {
-      this.map.updateSize()
-    }
+    const targetEl = this.map.getTargetElement()
+    if (targetEl) {
+      const [w, h] = [targetEl.clientWidth, targetEl.clientHeight]
+      const [cw, ch] = this.map.getSize()
+      if (w !== cw || h !== ch) {
+        this.map.updateSize()
+      }
+    }

If jQuery must stay, at least hoist the selection outside the loop and reuse it.

---

13-16: Optional: handle OpenLayers attribution/logo removal behind a flag.

If the customer decides to remove the OL attribution, add a small helper and a toggle to avoid hard‑coding legal choices.

   removeTileTools(map)
 
-  overrideMapViewUpdate(viewer)
+  // Optionally remove OL attribution/logo if permitted
+  // removeOlAttribution(map)
+
+  overrideMapViewUpdate(viewer)

Add helper:

function removeOlAttribution(map) {
  const toRemove = []
  map.getControls().forEach((c) => {
    const ctor = c && c.constructor && c.constructor.name
    if (ctor === 'Attribution') toRemove.push(c)
  })
  toRemove.forEach((c) => map.removeControl(c))
}

Confirm licensing before enabling by default.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro